refactor(bookings): delivers per-member round-robin limit overrides end-to-end #28715
Closed
manurana26770 wants to merge 19 commits intocalcom:mainfrom
Closed
refactor(bookings): delivers per-member round-robin limit overrides end-to-end #28715manurana26770 wants to merge 19 commits intocalcom:mainfrom
manurana26770 wants to merge 19 commits intocalcom:mainfrom
Conversation
…dividual-member-limits
|
|
github-actions
bot
added
consumer
High priority
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/features/users/lib/getRoutedUsers.ts">
<violation number="1" location="packages/features/users/lib/getRoutedUsers.ts:127">
P2: Large duplicated host-projection logic across booking paths increases drift risk for override/effective-limit fields.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Contributor
Author
|
@CarinaWolli could you plz review or tell anybody to review this. |
This was referenced Apr 6, 2026
Contributor
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
…feat/individual-member-limits
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds multiple per-host override fields (buffers, minimum booking notice, slot interval, booking/duration limits, and period configuration) across the Prisma schema, DB migration, backend types, repository selects, and UI components. It introduces utilities to compute per-host effective limits, group hosts by identical effective limits, and apply those buckets in availability, slot-generation, and round-robin booking-limit validation flows (including busy-time checks). It also updates host create/update mapping, user-loading credential handling, normalization/routing, and includes unit tests for round-robin effective-limit resolution. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/features/bookings/lib/handleNewBooking/resolveRoundRobinHostEffectiveLimits.ts (1)
122-157: Add a fast-path for non-round-robin scheduling to avoid redundant bucketing work.When scheduling is not round-robin, every host resolves to the same effective limits, so we can return a single bucket directly.
♻️ Proposed optimization
export function groupRoundRobinHostsByEffectiveLimits<THost>({ schedulingType, eventLimits, hosts, getHostOverrides, }: { schedulingType: SchedulingType | null; eventLimits: EventLevelLimits; hosts: THost[]; getHostOverrides: (host: THost) => RoundRobinHostLimitOverrides | null | undefined; }): EffectiveLimitBucket<THost>[] { + if (schedulingType !== SchedulingType.ROUND_ROBIN) { + const effectiveLimits = { ...eventLimits }; + return [ + { + profileKey: buildEffectiveHostLimitsProfileKey(effectiveLimits), + effectiveLimits, + hosts: [...hosts], + }, + ]; + } + const bucketsByKey = new Map<string, EffectiveLimitBucket<THost>>();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/features/bookings/lib/handleNewBooking/resolveRoundRobinHostEffectiveLimits.ts` around lines 122 - 157, Add a fast-path at the top of groupRoundRobinHostsByEffectiveLimits: when schedulingType is not the ROUND_ROBIN variant, compute effectiveLimits once by calling resolveRoundRobinHostEffectiveLimits with schedulingType and eventLimits (use a null/undefined hostOverrides since overrides are irrelevant for non-round-robin), build the profileKey via buildEffectiveHostLimitsProfileKey, and return a single EffectiveLimitBucket containing that effectiveLimits and the full hosts array instead of iterating and bucketing per host; keep references to resolveRoundRobinHostEffectiveLimits, buildEffectiveHostLimitsProfileKey, getHostOverrides and schedulingType to locate the relevant code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts`:
- Around line 144-154: The query in getEventTypesFromDB.ts selects many override
flags but omits the event-level slotInterval, causing hosts without
overrideSlotInterval to get undefined; update the select list to include the
eventType.slotInterval (the base slotInterval column) so the round-robin
effective-limit logic can fall back to eventType.slotInterval when
overrideSlotInterval is false/absent and functions that compute effective limits
or bucketing (where overrideSlotInterval and eventType.slotInterval are
consulted) will receive the correct value.
---
Nitpick comments:
In
`@packages/features/bookings/lib/handleNewBooking/resolveRoundRobinHostEffectiveLimits.ts`:
- Around line 122-157: Add a fast-path at the top of
groupRoundRobinHostsByEffectiveLimits: when schedulingType is not the
ROUND_ROBIN variant, compute effectiveLimits once by calling
resolveRoundRobinHostEffectiveLimits with schedulingType and eventLimits (use a
null/undefined hostOverrides since overrides are irrelevant for
non-round-robin), build the profileKey via buildEffectiveHostLimitsProfileKey,
and return a single EffectiveLimitBucket containing that effectiveLimits and the
full hosts array instead of iterating and bucketing per host; keep references to
resolveRoundRobinHostEffectiveLimits, buildEffectiveHostLimitsProfileKey,
getHostOverrides and schedulingType to locate the relevant code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 60c676c8-67dd-40aa-b88c-631dcb4e7936
📒 Files selected for processing (9)
packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.tspackages/features/bookings/lib/handleNewBooking/loadUsers.tspackages/features/bookings/lib/handleNewBooking/resolveRoundRobinHostEffectiveLimits.tspackages/features/bookings/lib/handleNewBooking/test/resolveRoundRobinHostEffectiveLimits.test.tspackages/features/bookings/lib/handleNewBooking/types.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/users/lib/getRoutedUsers.tspackages/prisma/migrations/20260402201939_add_host_round_robin_limit_overrides/migration.sqlpackages/prisma/schema.prisma
…dividual-member-limits
manurana26770
changed the title
…nurana26770/cal.com into feat/individual-member-limits
Contributor
Author
|
@PeerRich could you please review this or tell the desired person to review ? |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/modules/event-types/components/tabs/assignment/EventTeamAssignmentTab.tsx (1)
688-737:⚠️ Potential issue | 🟠 MajorRestore
existingHost.locationin this merge as well.
CheckedHostFieldround-trips hosts through a reduced option shape that does not carrylocation. BecauseupdatedHosts()only restoresscheduleId,groupId, and the new override fields, any priority/weight/limit edit can still clear an existing per-host location from form state. GivenenablePerHostLocationsalready exists, this is user-facing data loss.Suggested fix
return existingHost ? { ...newValue, scheduleId: existingHost.scheduleId, groupId: existingHost.groupId, + location: + newValue.location !== undefined ? newValue.location : existingHost.location, overrideMinimumBookingNotice: newValue.overrideMinimumBookingNotice !== undefined ? newValue.overrideMinimumBookingNotice : existingHost.overrideMinimumBookingNotice,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/modules/event-types/components/tabs/assignment/EventTeamAssignmentTab.tsx` around lines 688 - 737, The merge that returns the updated host object (the code branching that returns {...newValue, scheduleId: existingHost.scheduleId, groupId: existingHost.groupId, ...}) is not restoring existingHost.location, so edits can accidentally clear per-host location; update that return to include location: existingHost.location (unless newValue.location is intentionally set) — i.e., when composing the merged object in EventTeamAssignmentTab (the block referencing existingHost and newValue), preserve existingHost.location the same way scheduleId and groupId are preserved (use newValue.location if provided, otherwise fall back to existingHost.location).
🧹 Nitpick comments (3)
packages/trpc/server/routers/viewer/eventTypes/heavy/duplicate.handler.ts (1)
125-148: Extract this host-create mapping into a shared helper.
update.handler.tsalready contains the canonical create-path normalization for host overrides. Keeping another inline map here means collectiveisFixedrules, JSON-null handling, and future override fields have to stay in sync manually.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/trpc/server/routers/viewer/eventTypes/heavy/duplicate.handler.ts` around lines 125 - 148, The host-create mapping in duplicate.handler.ts duplicates the normalization logic from update.handler.ts; extract that mapping into a shared helper (e.g., normalizeHostCreatePayload or buildHostCreateData) and replace the inline hosts.map(...) in duplicate.handler.ts with a call to this helper, then update update.handler.ts to use the same helper for its canonical create-path normalization so JSON-null handling, isFixed rules, and any future override fields remain centralized and consistent across both handlers.packages/features/bookings/lib/handleNewBooking/ensureAvailableUsers.ts (1)
63-113: Move the effective-limit event helpers to a shared module.
getEventLevelLimits()andbuildEventTypeWithEffectiveLimits()are duplicated again inRegularBookingService.ts. That creates a quiet drift risk between availability filtering and final booking enforcement the next time a limit field is added or renamed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/features/bookings/lib/handleNewBooking/ensureAvailableUsers.ts` around lines 63 - 113, Extract getEventLevelLimits and buildEventTypeWithEffectiveLimits into a shared helper module and export them (preserving the getEventTypeResponse and IsFixedAwareUser types used in their signatures), then replace the duplicate definitions in ensureAvailableUsers and RegularBookingService with imports from that new module; ensure both callers use the exported functions and that the exported types/ReturnType match the original return shape so no downstream type changes are needed.packages/trpc/server/routers/viewer/slots/util.ts (1)
1732-1763: Consider extracting repeated slot generation logic.The first-two-weeks slot calculation (lines 1732-1763) duplicates the pattern from the main slot generation (lines 1671-1700). While functionally correct, this could be extracted into a shared helper to reduce code duplication.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/trpc/server/routers/viewer/slots/util.ts` around lines 1732 - 1763, The two slot-generation branches for firstTwoWeeksTimeSlots and the main slot generation duplicate logic; extract a shared helper (e.g., generateSlotsForHosts) that accepts parameters like startTime (from getStartTimeForHosts), eventType, input, eligible hosts list, and either uses getRoundRobinOverrideAwareSlots or getSlots internally based on hasRoundRobinHostLimitOverrides; replace both usages with calls to this new helper and reuse helpers like getAggregatedAvailability and getStartTimeForHosts inside it to keep behavior identical and remove duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/features/bookings/lib/host-filtering/filterHostsByLeadThreshold.ts`:
- Line 21: The change made to filterHostsByLeadThreshold currently treats a null
createdAt as an allow-all fallback, which breaks calibration in getLuckyUser
because allRRHosts[].createdAt is assumed present; update
filterHostsByLeadThreshold so it fails closed: do not return the original hosts
when any allRRHosts[].createdAt is null—instead either (a) filter out hosts with
null createdAt before applying maxLeadThreshold, or (b) throw/return an empty
list so the threshold is enforced; ensure the logic around symbols allRRHosts,
createdAt, and maxLeadThreshold in filterHostsByLeadThreshold (and related
checks at the other mentioned locations) enforces non-null createdAt rather than
bypassing the rule.
In `@packages/features/eventtypes/components/dialogs/HostEditDialogs.tsx`:
- Around line 335-340: The code is conflating "use event default" with an
explicit UNLIMITED override by using PeriodType.UNLIMITED as the placeholder for
no host override; change the host override state to use a distinct sentinel
(e.g., PeriodType.INHERIT or null/undefined) when initializing fields
(periodType, periodDays, periodCountCalendarDays, periodDates.startDate/endDate)
from option.override* so that PeriodType.UNLIMITED represents a real, explicit
unlimited override; update the reverse mapping/serialization logic (the code
that currently converts UNLIMITED back into null around the save/submit logic)
to convert the new INHERIT/null sentinel into null for downstream storage and
leave PeriodType.UNLIMITED intact as an explicit override; apply the same
pattern for all referenced fields and any helper functions that read/write
option.overridePeriod* values.
In `@packages/features/eventtypes/repositories/eventTypeRepository.ts`:
- Around line 1192-1203: findByIdMinimal currently selects only
overrideMinimumBookingNotice for eventType.hosts, causing
getRoundRobinHostLimitOverrides to receive undefined for the other override
fields; update the hosts projection inside findByIdMinimal to include all 11
override* fields (overrideMinimumBookingNotice, overrideBeforeEventBuffer,
overrideAfterEventBuffer, overrideSlotInterval, overrideBookingLimits,
overrideDurationLimits, overridePeriodType, overridePeriodDays,
overridePeriodCountCalendarDays, overridePeriodStartDate, overridePeriodEndDate)
so callers like isAvailable.handler (which calls
getRoundRobinHostLimitOverrides(host)) get a complete host object with all
required override properties.
In `@packages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.test.ts`:
- Around line 128-139: The test currently expects overrideBookingLimits and
overrideDurationLimits to be null but mapHostUpdateData() routes them through
toNullableJsonInput(), which converts null to Prisma.JsonNull; update the
assertion in update.handler.test.ts so the expected values for
overrideBookingLimits and overrideDurationLimits are Prisma.JsonNull (leave the
other override* fields as null) to match the behavior of toNullableJsonInput()
and mapHostUpdateData().
In `@packages/trpc/server/routers/viewer/slots/isAvailable.handler.ts`:
- Around line 47-70: The current computation of minimumBookingNotice incorrectly
uses Math.min for ROUND_ROBIN which allows a slot if any single host's notice is
satisfied; change the logic in the minimumBookingNotice calculation (the block
that checks eventType.schedulingType === SchedulingType.ROUND_ROBIN) to require
the maximum effective minimumBookingNotice across hosts instead of the
minimum—i.e., map hosts through
resolveRoundRobinHostEffectiveLimits(...).minimumBookingNotice (using
getRoundRobinHostLimitOverrides for hostOverrides) and take Math.max(...) (or
fold with Math.max) so the returned minimumBookingNotice enforces every required
host's notice window.
---
Outside diff comments:
In
`@apps/web/modules/event-types/components/tabs/assignment/EventTeamAssignmentTab.tsx`:
- Around line 688-737: The merge that returns the updated host object (the code
branching that returns {...newValue, scheduleId: existingHost.scheduleId,
groupId: existingHost.groupId, ...}) is not restoring existingHost.location, so
edits can accidentally clear per-host location; update that return to include
location: existingHost.location (unless newValue.location is intentionally set)
— i.e., when composing the merged object in EventTeamAssignmentTab (the block
referencing existingHost and newValue), preserve existingHost.location the same
way scheduleId and groupId are preserved (use newValue.location if provided,
otherwise fall back to existingHost.location).
---
Nitpick comments:
In `@packages/features/bookings/lib/handleNewBooking/ensureAvailableUsers.ts`:
- Around line 63-113: Extract getEventLevelLimits and
buildEventTypeWithEffectiveLimits into a shared helper module and export them
(preserving the getEventTypeResponse and IsFixedAwareUser types used in their
signatures), then replace the duplicate definitions in ensureAvailableUsers and
RegularBookingService with imports from that new module; ensure both callers use
the exported functions and that the exported types/ReturnType match the original
return shape so no downstream type changes are needed.
In `@packages/trpc/server/routers/viewer/eventTypes/heavy/duplicate.handler.ts`:
- Around line 125-148: The host-create mapping in duplicate.handler.ts
duplicates the normalization logic from update.handler.ts; extract that mapping
into a shared helper (e.g., normalizeHostCreatePayload or buildHostCreateData)
and replace the inline hosts.map(...) in duplicate.handler.ts with a call to
this helper, then update update.handler.ts to use the same helper for its
canonical create-path normalization so JSON-null handling, isFixed rules, and
any future override fields remain centralized and consistent across both
handlers.
In `@packages/trpc/server/routers/viewer/slots/util.ts`:
- Around line 1732-1763: The two slot-generation branches for
firstTwoWeeksTimeSlots and the main slot generation duplicate logic; extract a
shared helper (e.g., generateSlotsForHosts) that accepts parameters like
startTime (from getStartTimeForHosts), eventType, input, eligible hosts list,
and either uses getRoundRobinOverrideAwareSlots or getSlots internally based on
hasRoundRobinHostLimitOverrides; replace both usages with calls to this new
helper and reuse helpers like getAggregatedAvailability and getStartTimeForHosts
inside it to keep behavior identical and remove duplication.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 32898e10-7fb7-4318-a461-8c84e39edfbb
📒 Files selected for processing (25)
apps/web/modules/event-types/components/AddMembersWithSwitch.tsxapps/web/modules/event-types/components/EventType.tsxapps/web/modules/event-types/components/tabs/assignment/EventTeamAssignmentTab.tsxpackages/features/bookings/lib/handleNewBooking/ensureAvailableUsers.tspackages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.tspackages/features/bookings/lib/handleNewBooking/loadAndValidateUsers.tspackages/features/bookings/lib/handleNewBooking/loadUsers.tspackages/features/bookings/lib/handleNewBooking/resolveRoundRobinHostEffectiveLimits.tspackages/features/bookings/lib/host-filtering/filterHostsByLeadThreshold.tspackages/features/bookings/lib/host-filtering/findQualifiedHostsWithDelegationCredentials.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/ee/round-robin/roundRobinManualReassignment.tspackages/features/ee/round-robin/roundRobinReassignment.tspackages/features/eventtypes/components/CheckedTeamSelect.tsxpackages/features/eventtypes/components/dialogs/HostEditDialogs.tsxpackages/features/eventtypes/lib/types.tspackages/features/eventtypes/repositories/eventTypeRepository.tspackages/features/users/lib/getRoutedUsers.tspackages/i18n/locales/en/common.jsonpackages/trpc/server/routers/viewer/eventTypes/heavy/duplicate.handler.tspackages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.test.tspackages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.tspackages/trpc/server/routers/viewer/eventTypes/types.tspackages/trpc/server/routers/viewer/slots/isAvailable.handler.tspackages/trpc/server/routers/viewer/slots/util.ts
✅ Files skipped from review due to trivial changes (5)
- packages/features/ee/round-robin/roundRobinReassignment.ts
- packages/features/ee/round-robin/roundRobinManualReassignment.ts
- packages/i18n/locales/en/common.json
- packages/features/bookings/lib/host-filtering/findQualifiedHostsWithDelegationCredentials.ts
- apps/web/modules/event-types/components/EventType.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/features/users/lib/getRoutedUsers.ts
- packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts
- packages/features/bookings/lib/handleNewBooking/loadUsers.ts
- packages/features/bookings/lib/handleNewBooking/resolveRoundRobinHostEffectiveLimits.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/modules/event-types/components/tabs/assignment/EventTeamAssignmentTab.tsx (1)
170-190:⚠️ Potential issue | 🟠 MajorPreserve
locationwhen these activation paths rebuild hosts.These mappers now keep the new
override*fields, but they still recreate each host withoutlocation. Toggling fixed-host activation or RR member activation will therefore clear any previously configured per-host location.Suggested fix
overridePeriodEndDate: host?.overridePeriodEndDate, overridePeriodDays: host?.overridePeriodDays, overridePeriodCountCalendarDays: host?.overridePeriodCountCalendarDays, + location: host?.location, // if host was already added, retain scheduleId and groupId scheduleId: host?.scheduleId || teamMember.defaultScheduleId, groupId: host?.groupId || null,overridePeriodEndDate: host?.overridePeriodEndDate, overridePeriodDays: host?.overridePeriodDays, overridePeriodCountCalendarDays: host?.overridePeriodCountCalendarDays, + location: host?.location, // if host was already added, retain scheduleId and groupId scheduleId: host?.scheduleId || teamMember.defaultScheduleId, groupId: host?.groupId || groupId,Also applies to: 407-427
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/modules/event-types/components/tabs/assignment/EventTeamAssignmentTab.tsx` around lines 170 - 190, The mapper in EventTeamAssignmentTab (the teamMembers.map callback that looks up host via currentHosts.find) rebuilds host objects but omits the existing location, which wipes per-host location on reactivation; update the returned object to preserve location from the existing host (e.g., location: host?.location || teamMember.defaultLocationOrNull) in the same manner as scheduleId/groupId so existing host.location is retained, and apply the same change to the other equivalent mapper block referenced around lines 407-427.
♻️ Duplicate comments (1)
packages/features/bookings/lib/host-filtering/filterHostsByLeadThreshold.ts (1)
123-133:⚠️ Potential issue | 🟠 MajorReturning
[]still re-enables the allow-all fallback.When every host is missing
createdAt, this path returns an empty array, but the caller still wraps this helper withapplyFilterWithFallback(...). In that all-null case, the original host list gets restored andmaxLeadThresholdis skipped again. Please fail closed in a way the caller will not treat as “no matches,” or move the null-createdAtfiltering before that fallback wrapper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/features/bookings/lib/host-filtering/filterHostsByLeadThreshold.ts` around lines 123 - 133, The current early return of [] when hostsWithCreatedAt.length === 0 causes the caller (which uses applyFilterWithFallback) to interpret this as “no matches” and re-enable the allow-all fallback; instead fail closed by returning the original hosts list so the caller sees a non-empty result and will not trigger the fallback. Update filterHostsByLeadThreshold: when hostsWithCreatedAt is empty (after hasCreatedAt filtering) return hosts (the original array) rather than [] — or alternatively move the hasCreatedAt pre-filtering step out so it runs before the applyFilterWithFallback wrapper; reference symbols: filterHostsByLeadThreshold, hostsWithCreatedAt, hasCreatedAt, applyFilterWithFallback.
🧹 Nitpick comments (2)
packages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.test.ts (1)
6-6: Import the mapper helpers from./hostDataMappingdirectly.This is a unit test for the mapping helpers, but importing them through
update.handler.tspulls the whole handler module and its top-level dependencies into the test. A direct import keeps the test narrower and less brittle.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.test.ts` at line 6, The test is importing mapHostCreateData and mapHostUpdateData via "./update.handler", which pulls in the entire handler module; change the import to pull the helpers directly from their module (hostDataMapping) by importing mapHostCreateData and mapHostUpdateData from "./hostDataMapping" so the unit test only exercises the mapping logic and avoids loading update.handler's top-level dependencies.packages/features/bookings/lib/service/RegularBookingService.ts (1)
1141-1158: ReusegetEventLevelLimits()here.This object literal duplicates the shared mapper added in the same PR. If one of these limit fields changes later, host bucketing here can drift from the per-host validation path below and from slots.
♻️ Suggested cleanup
const roundRobinLimitBuckets = groupRoundRobinHostsByEffectiveLimits({ schedulingType: eventType.schedulingType, - eventLimits: { - minimumBookingNotice: eventType.minimumBookingNotice, - beforeEventBuffer: eventType.beforeEventBuffer, - afterEventBuffer: eventType.afterEventBuffer, - slotInterval: eventType.slotInterval, - bookingLimits: eventType.bookingLimits, - durationLimits: eventType.durationLimits, - periodType: eventType.periodType, - periodDays: eventType.periodDays, - periodCountCalendarDays: eventType.periodCountCalendarDays, - periodStartDate: eventType.periodStartDate, - periodEndDate: eventType.periodEndDate, - }, + eventLimits: getEventLevelLimits(eventType), hosts: nonFixedUsers, getHostOverrides: getRoundRobinHostLimitOverrides, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/features/bookings/lib/service/RegularBookingService.ts` around lines 1141 - 1158, The eventLimits object is duplicated here and should be reused via getEventLevelLimits to avoid drift; replace the inline eventLimits: { ... } with eventLimits: getEventLevelLimits(eventType) while keeping schedulingType: eventType.schedulingType and the other arguments (hosts: nonFixedUsers, getHostOverrides: getRoundRobinHostLimitOverrides) intact when calling groupRoundRobinHostsByEffectiveLimits so host bucketing uses the same shared mapper as per-host validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/features/eventtypes/components/dialogs/HostEditDialogs.tsx`:
- Around line 381-489: applyOverrides currently builds the emitted host list
from rrHosts only, dropping fixed hosts; update applyOverrides to preserve the
untouched fixed-host slice by merging it back into the final array instead of
emitting only otherGroupsOptions and updatedOptions. Specifically, use the
original hosts (getValues("hosts")) to derive fixedHosts = hosts.filter(h =>
h.isFixed) or construct the finalHosts by taking all original hosts and
replacing only the members of the targeted group (option.groupId and
option.value) with the updated versions from sortedHostGroup/updatedOptions (or
by concatenating fixedHosts + otherGroupsOptions + updatedOptions while ensuring
no duplicates), then call onChange(finalHosts) and close the dialog; keep
references to applyOverrides, rrHosts, otherGroupsHosts/getHostsFromOtherGroups,
sortedHostGroup/updatedOptions and option to locate the code to change.
In `@packages/trpc/server/routers/viewer/slots/util.ts`:
- Around line 1307-1379: The current loop still calls
userAvailabilityService.getUserAvailability per user, causing N calendar
fetches; instead, for each bucket returned by
groupRoundRobinHostsByEffectiveLimits (where bucket.hosts share identical
effectiveLimits) call getUserAvailability once for the whole bucket and then
distribute its results to each host. Concretely, replace the users.map(...) that
invokes getUserAvailability(user) with a bucket-level call: for each bucket
compute effectiveLimits (from bucket.effectiveLimits), call getUserAvailability
once passing the bucket-level params
(dateFrom/dateTo/duration/afterEventBuffer/beforeEventBuffer/bypassBusyCalendarTimes/etc.)
and include busyTimesFromLimitsBookings for all bucket.hosts (use
busyTimesFromLimitsByUserId to assemble per-user busy times if the service
accepts multi-user input), then map the returned availability entries back to
individual users in bucket.hosts (using user.id) so callers get per-user
availability without N separate calendar fetches; update any call sites
expecting a single-user signature to the batched form or adapt the returned
structure accordingly.
---
Outside diff comments:
In
`@apps/web/modules/event-types/components/tabs/assignment/EventTeamAssignmentTab.tsx`:
- Around line 170-190: The mapper in EventTeamAssignmentTab (the teamMembers.map
callback that looks up host via currentHosts.find) rebuilds host objects but
omits the existing location, which wipes per-host location on reactivation;
update the returned object to preserve location from the existing host (e.g.,
location: host?.location || teamMember.defaultLocationOrNull) in the same manner
as scheduleId/groupId so existing host.location is retained, and apply the same
change to the other equivalent mapper block referenced around lines 407-427.
---
Duplicate comments:
In `@packages/features/bookings/lib/host-filtering/filterHostsByLeadThreshold.ts`:
- Around line 123-133: The current early return of [] when
hostsWithCreatedAt.length === 0 causes the caller (which uses
applyFilterWithFallback) to interpret this as “no matches” and re-enable the
allow-all fallback; instead fail closed by returning the original hosts list so
the caller sees a non-empty result and will not trigger the fallback. Update
filterHostsByLeadThreshold: when hostsWithCreatedAt is empty (after hasCreatedAt
filtering) return hosts (the original array) rather than [] — or alternatively
move the hasCreatedAt pre-filtering step out so it runs before the
applyFilterWithFallback wrapper; reference symbols: filterHostsByLeadThreshold,
hostsWithCreatedAt, hasCreatedAt, applyFilterWithFallback.
---
Nitpick comments:
In `@packages/features/bookings/lib/service/RegularBookingService.ts`:
- Around line 1141-1158: The eventLimits object is duplicated here and should be
reused via getEventLevelLimits to avoid drift; replace the inline eventLimits: {
... } with eventLimits: getEventLevelLimits(eventType) while keeping
schedulingType: eventType.schedulingType and the other arguments (hosts:
nonFixedUsers, getHostOverrides: getRoundRobinHostLimitOverrides) intact when
calling groupRoundRobinHostsByEffectiveLimits so host bucketing uses the same
shared mapper as per-host validation.
In `@packages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.test.ts`:
- Line 6: The test is importing mapHostCreateData and mapHostUpdateData via
"./update.handler", which pulls in the entire handler module; change the import
to pull the helpers directly from their module (hostDataMapping) by importing
mapHostCreateData and mapHostUpdateData from "./hostDataMapping" so the unit
test only exercises the mapping logic and avoids loading update.handler's
top-level dependencies.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 303c7746-fbeb-4ce0-956c-7be2d96afbd0
📒 Files selected for processing (13)
apps/web/modules/event-types/components/tabs/assignment/EventTeamAssignmentTab.tsxpackages/features/bookings/lib/handleNewBooking/ensureAvailableUsers.tspackages/features/bookings/lib/handleNewBooking/eventTypeEffectiveLimits.tspackages/features/bookings/lib/host-filtering/filterHostsByLeadThreshold.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/eventtypes/components/dialogs/HostEditDialogs.tsxpackages/features/eventtypes/repositories/eventTypeRepository.tspackages/trpc/server/routers/viewer/eventTypes/heavy/duplicate.handler.tspackages/trpc/server/routers/viewer/eventTypes/heavy/hostDataMapping.tspackages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.test.tspackages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.tspackages/trpc/server/routers/viewer/slots/isAvailable.handler.tspackages/trpc/server/routers/viewer/slots/util.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/trpc/server/routers/viewer/slots/isAvailable.handler.ts
- packages/features/eventtypes/repositories/eventTypeRepository.ts
|
@manurana26770 is attempting to deploy a commit to the cal-diy Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
packages/features/bookings/lib/host-filtering/filterHostsByLeadThreshold.ts (1)
123-135: Pre-filtering logic correctly handles mixed pools; minor clarity suggestion for logs.The filtering approach now properly excludes hosts without
createdAtfrom the final result in mixed pools (fail closed), addressing the threshold bypass concern. The graceful fallback when all hosts lackcreatedAtis reasonable to avoid breaking booking flow entirely.However, when all hosts lack
createdAt, both log messages fire in sequence, which may read contradictorily ("filtering to enforce" then "returning originals"). Consider adjusting the first log to only fire whenhostsWithCreatedAt.length > 0after the filter, or rewording for clarity.💡 Optional: Clearer logging when all hosts lack createdAt
const hostsWithCreatedAt = hosts.filter(hasCreatedAt); - if (hostsWithCreatedAt.length !== hosts.length) { + if (hostsWithCreatedAt.length > 0 && hostsWithCreatedAt.length !== hosts.length) { log.debug( "Filtering out round-robin hosts missing createdAt before lead-threshold computation to enforce maxLeadThreshold" ); } if (hostsWithCreatedAt.length === 0) { log.debug( - "No round-robin hosts with createdAt available after lead-threshold pre-filtering; returning original hosts to avoid fallback re-enabling" + "All round-robin hosts lack createdAt; skipping lead-threshold filter and returning original hosts" ); return hosts; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/features/bookings/lib/host-filtering/filterHostsByLeadThreshold.ts` around lines 123 - 135, The two consecutive log.debug messages in filterHostsByLeadThreshold.ts are confusing when every host lacks createdAt; update the first debug so it only logs when at least one host remains after filtering: after computing hostsWithCreatedAt = hosts.filter(hasCreatedAt), wrap the first log.debug call in a condition hostsWithCreatedAt.length > 0 (or modify its text to indicate "some" hosts were filtered), leaving the second debug and the early return unchanged so behavior stays the same but logs no longer contradict; refer to hostsWithCreatedAt, hasCreatedAt, log.debug and the function filterHostsByLeadThreshold when making the change.packages/trpc/server/routers/viewer/slots/util.ts (1)
1389-1394: Index-based mapping assumes array order preservation.The mapping between
bucket.hostsandbucketAvailabilityrelies ongetUsersAvailabilityreturning results in the same order as the inputusersarray. While this is typically true, it's implicit and could break if the underlying implementation changes.Consider using a more explicit mapping by user ID, or adding a comment documenting this ordering assumption.
♻️ Suggested alternative using explicit ID mapping
- bucket.hosts.forEach((user, index) => { - const availability = bucketAvailability[index]; - if (availability) { - availabilityByUserId.set(user.id, availability); - } - }); + // Note: getUsersAvailability returns results in same order as input users + bucket.hosts.forEach((user, index) => { + const availability = bucketAvailability[index]; + if (availability) { + availabilityByUserId.set(user.id, availability); + } + });Or for more robustness, if
bucketAvailabilityitems contain user reference:bucketAvailability.forEach((availability, index) => { const user = bucket.hosts[index]; if (user && availability) { availabilityByUserId.set(user.id, availability); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/trpc/server/routers/viewer/slots/util.ts` around lines 1389 - 1394, The code assumes bucket.hosts and bucketAvailability align by index (see bucket.hosts, bucketAvailability, getUsersAvailability, availabilityByUserId); make the mapping explicit by matching availability items to users by ID instead of index (or add a clear comment documenting the ordering contract from getUsersAvailability). Update the loop that currently uses bucket.hosts.forEach(...) to either iterate bucketAvailability and look up the corresponding user by id or construct a userId->user map and use it to set availabilityByUserId, ensuring no implicit index ordering is relied upon.packages/features/eventtypes/components/dialogs/HostEditDialogs.tsx (1)
82-92: Extract a shared host-to-option mapper to prevent override-field drift.The same override field list is repeated in multiple mapping blocks. This is easy to desync during future additions (new override fields).
♻️ Refactor sketch
+const mapHostToCheckedOption = ({ + host, + userOption, + fallbackOption, + isFixedOverride, +}: { + host: Host; + userOption?: CheckedSelectOption; + fallbackOption: CheckedSelectOption; + isFixedOverride?: boolean; +}): CheckedSelectOption => ({ + ...fallbackOption, + avatar: userOption?.avatar ?? "", + label: userOption?.label ?? host.userId.toString(), + value: host.userId.toString(), + priority: host.priority, + weight: host.weight, + isFixed: isFixedOverride ?? host.isFixed, + groupId: host.groupId, + userId: host.userId, + overrideMinimumBookingNotice: host.overrideMinimumBookingNotice, + overrideBeforeEventBuffer: host.overrideBeforeEventBuffer, + overrideAfterEventBuffer: host.overrideAfterEventBuffer, + overrideSlotInterval: host.overrideSlotInterval, + overrideBookingLimits: host.overrideBookingLimits, + overrideDurationLimits: host.overrideDurationLimits, + overridePeriodType: host.overridePeriodType, + overridePeriodStartDate: host.overridePeriodStartDate, + overridePeriodEndDate: host.overridePeriodEndDate, + overridePeriodDays: host.overridePeriodDays, + overridePeriodCountCalendarDays: host.overridePeriodCountCalendarDays, +});Also applies to: 109-119, 213-223, 239-249, 450-460, 474-484, 501-511
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/features/eventtypes/components/dialogs/HostEditDialogs.tsx` around lines 82 - 92, The repeated override-field mapping in HostEditDialogs.tsx should be extracted into a single reusable mapper to avoid drift: create a function (e.g., mapHostOverrides or buildOverrideOptions) or a shared constant list (e.g., OVERRIDE_FIELDS) that enumerates the override keys (overrideMinimumBookingNotice, overrideBeforeEventBuffer, overrideAfterEventBuffer, overrideSlotInterval, overrideBookingLimits, overrideDurationLimits, overridePeriodType, overridePeriodStartDate, overridePeriodEndDate, overridePeriodDays, overridePeriodCountCalendarDays) and use it from each place that currently copies those fields (the mapping blocks in HostEditDialogs.tsx) to produce the same object shape (e.g., via reduce or a simple loop), then replace each duplicated mapping with a call to that function to ensure future additions only need to be updated in one place.apps/web/modules/event-types/components/tabs/assignment/EventTeamAssignmentTab.tsx (1)
177-191: Extract the host override/location merge into one helper.This field list is now hand-maintained in three places. The next per-host setting will be easy to miss in one path and reintroduce drift. A shared helper for “preserve existing host settings unless explicitly changed” would make this much safer.
Also applies to: 415-429, 695-739
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/modules/event-types/components/tabs/assignment/EventTeamAssignmentTab.tsx` around lines 177 - 191, Extract the repeated host-override merging logic into a single helper (e.g., preserveHostSettings or mergeHostOverrides) that accepts host and teamMember and returns the merged fields (overrideMinimumBookingNotice, overrideBeforeEventBuffer, overrideAfterEventBuffer, overrideSlotInterval, overrideBookingLimits, overrideDurationLimits, overridePeriodType, overridePeriodStartDate, overridePeriodEndDate, overridePeriodDays, overridePeriodCountCalendarDays, scheduleId, groupId, location). Replace the in-place object assembly in EventTeamAssignmentTab (the blocks that read host?...., scheduleId: host?.scheduleId || teamMember.defaultScheduleId, groupId: host?.groupId || null, location: host?.location ?? null) with a call to this helper, and reuse it for the other duplicate blocks in the same component to ensure consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/features/bookings/lib/service/RegularBookingService.ts`:
- Around line 1318-1399: Summary: Selected round-robin hosts with conflicting
effective limits cause an immediate 403 instead of falling back to the next
candidate. Fix: preserve effective-limit buckets through selection or add retry
logic when a chosen host fails limit validation: when calling getLuckyUser (or
after it returns), iterate remaining candidates in the same luckyUserPools group
(or update getLuckyUser to return a candidate iterator) and for each candidate
run resolveRoundRobinHostEffectiveLimits + buildEventTypeWithEffectiveLimits +
validateBookingTimeIsNotOutOfBounds + limit checks (the block using
parseBookingLimit/parseDurationLimit/getBusyTimesForLimitChecks/getBusyTimesFromLimits/doesBookingConflictWithBusyTimes).
Only throw the 403 after exhausting that bucket; ensure luckyUserPools grouping
keys (groupId) and effectiveLimits from resolveRoundRobinHostEffectiveLimits are
used to keep bucket boundaries during retries.
In `@packages/features/eventtypes/components/dialogs/HostEditDialogs.tsx`:
- Around line 314-320: The toUtcMidnight function is using local date getters
which causes timezone drift when dates are normalized twice (once in
onDatesChange and again before persistence); update toUtcMidnight to use UTC
getters (getUTCFullYear, getUTCMonth, getUTCDate) instead of
getFullYear/getMonth/getDate so the function produces a true UTC-midnight Date
and prevents the stored date from shifting in non-UTC timezones; locate and
change the toUtcMidnight function declaration to use those UTC methods wherever
it constructs the Date.UTC value.
In `@packages/trpc/server/routers/viewer/slots/util.ts`:
- Around line 2047-2063: The code references eventTimeZone inside
_getAvailableSlots but that variable is not defined in this scope; define it
before it's used (e.g., const eventTimeZone = event?.timeZone ??
eventType?.timeZone) so getUTCOffsetByTimezone(eventTimeZone) receives the
actual event timezone; add this definition near the start of _getAvailableSlots
(before the hasRoundRobinHostLimitOverrides block) and ensure it uses available
symbols like event and eventType so period limit calculations use the correct
timezone.
---
Nitpick comments:
In
`@apps/web/modules/event-types/components/tabs/assignment/EventTeamAssignmentTab.tsx`:
- Around line 177-191: Extract the repeated host-override merging logic into a
single helper (e.g., preserveHostSettings or mergeHostOverrides) that accepts
host and teamMember and returns the merged fields (overrideMinimumBookingNotice,
overrideBeforeEventBuffer, overrideAfterEventBuffer, overrideSlotInterval,
overrideBookingLimits, overrideDurationLimits, overridePeriodType,
overridePeriodStartDate, overridePeriodEndDate, overridePeriodDays,
overridePeriodCountCalendarDays, scheduleId, groupId, location). Replace the
in-place object assembly in EventTeamAssignmentTab (the blocks that read
host?...., scheduleId: host?.scheduleId || teamMember.defaultScheduleId,
groupId: host?.groupId || null, location: host?.location ?? null) with a call to
this helper, and reuse it for the other duplicate blocks in the same component
to ensure consistency.
In `@packages/features/bookings/lib/host-filtering/filterHostsByLeadThreshold.ts`:
- Around line 123-135: The two consecutive log.debug messages in
filterHostsByLeadThreshold.ts are confusing when every host lacks createdAt;
update the first debug so it only logs when at least one host remains after
filtering: after computing hostsWithCreatedAt = hosts.filter(hasCreatedAt), wrap
the first log.debug call in a condition hostsWithCreatedAt.length > 0 (or modify
its text to indicate "some" hosts were filtered), leaving the second debug and
the early return unchanged so behavior stays the same but logs no longer
contradict; refer to hostsWithCreatedAt, hasCreatedAt, log.debug and the
function filterHostsByLeadThreshold when making the change.
In `@packages/features/eventtypes/components/dialogs/HostEditDialogs.tsx`:
- Around line 82-92: The repeated override-field mapping in HostEditDialogs.tsx
should be extracted into a single reusable mapper to avoid drift: create a
function (e.g., mapHostOverrides or buildOverrideOptions) or a shared constant
list (e.g., OVERRIDE_FIELDS) that enumerates the override keys
(overrideMinimumBookingNotice, overrideBeforeEventBuffer,
overrideAfterEventBuffer, overrideSlotInterval, overrideBookingLimits,
overrideDurationLimits, overridePeriodType, overridePeriodStartDate,
overridePeriodEndDate, overridePeriodDays, overridePeriodCountCalendarDays) and
use it from each place that currently copies those fields (the mapping blocks in
HostEditDialogs.tsx) to produce the same object shape (e.g., via reduce or a
simple loop), then replace each duplicated mapping with a call to that function
to ensure future additions only need to be updated in one place.
In `@packages/trpc/server/routers/viewer/slots/util.ts`:
- Around line 1389-1394: The code assumes bucket.hosts and bucketAvailability
align by index (see bucket.hosts, bucketAvailability, getUsersAvailability,
availabilityByUserId); make the mapping explicit by matching availability items
to users by ID instead of index (or add a clear comment documenting the ordering
contract from getUsersAvailability). Update the loop that currently uses
bucket.hosts.forEach(...) to either iterate bucketAvailability and look up the
corresponding user by id or construct a userId->user map and use it to set
availabilityByUserId, ensuring no implicit index ordering is relied upon.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 25bbbd09-a700-4381-b1a8-6aa7b1823c47
📒 Files selected for processing (6)
apps/web/modules/event-types/components/tabs/assignment/EventTeamAssignmentTab.tsxpackages/features/bookings/lib/host-filtering/filterHostsByLeadThreshold.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/eventtypes/components/dialogs/HostEditDialogs.tsxpackages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.test.tspackages/trpc/server/routers/viewer/slots/util.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/features/eventtypes/components/dialogs/HostEditDialogs.tsx (1)
646-661: Extract repeated options arrays to avoid duplication.The period type options array is defined twice: once for the
optionsprop and again for thevaluelookup. This pattern is also repeated for the calendar/business days select (lines 690-699).♻️ Suggested refactor
+ const periodTypeOptions = [ + { label: t("unlimited"), value: PeriodType.UNLIMITED }, + { label: t("rolling"), value: PeriodType.ROLLING }, + { label: t("rolling_window"), value: PeriodType.ROLLING_WINDOW }, + { label: t("within_date_range"), value: PeriodType.RANGE }, + ]; + <Controller name="periodType" control={limitForm.control} render={({ field: { onChange, value } }) => ( <Select isSearchable={false} - options={[ - { label: t("unlimited"), value: PeriodType.UNLIMITED }, - { label: t("rolling"), value: PeriodType.ROLLING }, - { label: t("rolling_window"), value: PeriodType.ROLLING_WINDOW }, - { label: t("within_date_range"), value: PeriodType.RANGE }, - ]} + options={periodTypeOptions} onChange={(selected) => onChange(selected?.value)} - value={ - [ - { label: t("unlimited"), value: PeriodType.UNLIMITED }, - { label: t("rolling"), value: PeriodType.ROLLING }, - { label: t("rolling_window"), value: PeriodType.ROLLING_WINDOW }, - { label: t("within_date_range"), value: PeriodType.RANGE }, - ].find((periodOption) => periodOption.value === (value ?? PeriodType.UNLIMITED)) ?? null - } + value={periodTypeOptions.find((opt) => opt.value === (value ?? PeriodType.UNLIMITED)) ?? null} /> )} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/features/eventtypes/components/dialogs/HostEditDialogs.tsx` around lines 646 - 661, Extract the duplicated period options array into a single constant (e.g., const periodOptions = [{ label: t("unlimited"), value: PeriodType.UNLIMITED }, ...]) within the HostEditDialogs component and replace both the options prop and the value lookup (the .find(...) that compares periodOption.value === (value ?? PeriodType.UNLIMITED)) to use periodOptions; do the same refactor for the calendar/business days select by creating a shared constant for its options and reusing it for both its options prop and value resolution so you avoid repeating identical arrays.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/features/bookings/lib/service/RegularBookingService.ts`:
- Around line 1408-1429: The retry path builds orderedCandidates from
luckyUserPoolsForRetry but does not re-run the recurring-slot availability check
(ensureAvailableUsers), so fallback hosts can be chosen who fail availability on
later occurrences; update the retry logic around
orderedCandidates/chosenCandidate to either persist the recurring-eligible
candidate set into luckyUserPoolsForRetry when first computed or, before
accepting a candidate in the loop that uses getLimitConflictMessageForUser,
re-run the recurring availability check (ensureAvailableUsers or the same helper
used originally) for that candidate and skip them if they fail; ensure the
change touches the same selection block (orderedCandidates, chosenCandidate,
lastConflictMessage) so only recurring-eligible users are considered for
retries.
- Around line 1402-1447: In the luckyUserPoolsForRetry branch, validate fixed RR
hosts before carrying them straight into users: for each member in fixedUsers
call validateBookingTimeIsNotOutOfBounds(member) and then await
getLimitConflictMessageForUser(member); if validation fails, throw an HttpError
with statusCode 403 and the same conflict message pattern used later (use the
conflict string or "booking_limit_reached" as fallback). Do this prior to
building usedUserIds/resolvedLuckyUsers so fixed hosts are rejected the same way
non-fixed candidates are.
---
Nitpick comments:
In `@packages/features/eventtypes/components/dialogs/HostEditDialogs.tsx`:
- Around line 646-661: Extract the duplicated period options array into a single
constant (e.g., const periodOptions = [{ label: t("unlimited"), value:
PeriodType.UNLIMITED }, ...]) within the HostEditDialogs component and replace
both the options prop and the value lookup (the .find(...) that compares
periodOption.value === (value ?? PeriodType.UNLIMITED)) to use periodOptions; do
the same refactor for the calendar/business days select by creating a shared
constant for its options and reusing it for both its options prop and value
resolution so you avoid repeating identical arrays.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ebdc743a-9d26-4ca0-9a99-29849fa90e4e
📒 Files selected for processing (3)
packages/features/bookings/lib/service/RegularBookingService.tspackages/features/eventtypes/components/dialogs/HostEditDialogs.tsxpackages/trpc/server/routers/viewer/slots/util.ts
Member
|
Thank you for your contribution. This PR is being closed because it modifies code that no longer exists in Cal.diy. reference: blog |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR delivers per-member round-robin limit overrides end-to-end across event type configuration, booking execution, and slot computation.
What changes
Visual Demo (For contributors especially)
N/A for this foundation PR. The primary value is backend and routing plumbing plus assignment configuration support.
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Run the targeted unit tests:
TZ=UTC yarn vitest run packages/features/bookings/lib/handleNewBooking/test/resolveRoundRobinHostEffectiveLimits.test.ts
TZ=UTC yarn vitest run packages/features/bookings/lib/handleNewBooking/test/computeTeamData.test.ts
TZ=UTC yarn vitest run packages/features/bookings/lib/handleNewBooking/test/round-robin-no-hosts.test.ts
TZ=UTC yarn vitest run packages/features/users/lib/getRoutedUsers.test.ts
TZ=UTC yarn vitest run packages/trpc/server/routers/viewer/eventTypes/heavy/update.handler.test.ts
TZ=UTC yarn vitest run packages/trpc/server/routers/viewer/slots/util.test.ts
Expected result:
Event-level limits remain the default source of truth.
Host override values are resolved only for round-robin hosts.
When no host overrides exist, behavior stays exactly the same as before.
Routing host normalization preserves override fields consistently across relevant paths.
Slot host bucketing is deterministic for hosts with the same effective limit profile.
Optional manual validation:
In event type assignment, set host-specific override values and save.
Re-open assignment dialogs and verify values persist.
Verify booking/availability behavior:
unchanged when no host overrides are set
host-specific effective limits apply when overrides are set
unset host fields continue to fall back to event-level defaults
Checklist